combine json chunks from requests #4317

jiapingzeng · 2025-10-14T18:59:35Z

Description

[Describe what this change achieves]

Related Issues

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

nathaliellenaa · 2025-10-14T19:19:09Z

Thanks for the fix @jiapingzeng! Wondering if you see any additional latency due to combining the chunks?

jiapingzeng · 2025-10-14T19:30:14Z

I didn't notice additional latency from my side. There are some concerns of collectList() running out of memory when waiting for an infinite upstream: https://stackoverflow.com/a/72042404, but I don't think that applies here as agent execute requests are a few kBs at most.

jiapingzeng · 2025-10-15T04:13:22Z

Tests with failures:
 - org.opensearch.ml.rest.RestMLRAGSearchProcessorIT.testBM25WithBedrockWithConversation
131 tests completed, 1 failed, 11 skipped

=== Standard output of node `node{:opensearch-ml-plugin:integTest-0}` ===

»    ↓ errors and warnings from /__w/ml-commons/ml-commons/plugin/build/testclusters/integTest-0/logs/opensearch.stdout.log ↓
» WARN ][stderr                   ] [integTest-0] Oct 14, 2025 7:57:42 PM org.opensearch.javaagent.bootstrap.AgentPolicy setPolicy
» WARN ][stderr                   ] [integTest-0] INFO: Policy attached successfully: org.opensearch.bootstrap.OpenSearchPolicy@20de05e5
» WARN ][o.o.p.PluginsService     ] [integTest-0] Missing plugin [opensearch-security], dependency of [opensearch-ml]
» WARN ][o.o.p.PluginsService     ] [integTest-0] Some features of this plugin may not function without the dependencies being installed.
»  
» WARN ][stderr                   ] [integTest-0] Oct 14, 2025 7:57:42 PM org.apache.lucene.internal.vectorization.PanamaVectorizationProvider <init>
» WARN ][stderr                   ] [integTest-0] INFO: Java vector incubator API enabled; uses preferredBitSize=256; FMA enabled
» WARN ][o.o.g.DanglingIndicesState] [integTest-0] gateway.auto_import_dangling_indices is disabled, dangling indices will not be automatically detected or imported and must be managed manually
» WARN ][o.o.m.a.p.TransportPredictionStreamTaskAction] [integTest-0] StreamTransportService is not available.
» WARN ][o.o.m.a.e.TransportExecuteStreamTaskAction] [integTest-0] StreamTransportService is not available.
» WARN ][o.o.d.FileBasedSeedHostsProvider] [integTest-0] expected, but did not find, a dynamic hosts list at [/__w/ml-commons/ml-commons/plugin/build/testclusters/integTest-0/config/unicast_hosts.txt]
» WARN ][a.d.h.t.HuggingFaceTokenizer] [integTest-0] maxLength is not explicitly specified, use modelMaxLength: 512
» ERROR][o.o.m.a.MLModelAutoReDeployer] [integTest-0] Failed to query need undeploy models, no action will be performed
»  org.opensearch.index.IndexNotFoundException: no such index [.plugins-ml-model]
»  	at org.opensearch.cluster.metadata.IndexNameExpressionResolver$WildcardExpressionResolver.indexNotFoundException(IndexNameExpressionResolver.java:1101) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
»  	at org.opensearch.cluster.metadata.IndexNameExpressionResolver$WildcardExpressionResolver.innerResolve(IndexNameExpressionResolver.java:1038) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]
»  	at org.opensearch.cluster.metadata.IndexNameExpressionResolver$WildcardExpressionResolver.resolve(IndexNameExpressionResolver.java:994) ~[opensearch-3.3.0-SNAPSHOT.jar:3.3.0-SNAPSHOT]

Don't think test failure is related to my change. Tried ./gradlew build locally and it passes.

codecov · 2025-10-15T20:30:51Z

Codecov Report

❌ Patch coverage is 35.71429% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.14%. Comparing base (66bcde7) to head (8f6c270).

Files with missing lines	Patch %	Lines
.../opensearch/ml/rest/RestMLExecuteStreamAction.java	35.71%	9 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4317      +/-   ##
============================================
- Coverage     80.14%   80.14%   -0.01%     
- Complexity    10172    10174       +2     
============================================
  Files           854      854              
  Lines         44219    44227       +8     
  Branches       5113     5114       +1     
============================================
+ Hits          35440    35445       +5     
- Misses         6630     6633       +3     
  Partials       2149     2149

Flag	Coverage Δ
ml-commons	`80.14% <35.71%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jiapingzeng · 2025-10-21T18:55:05Z

Looks like CI is passing on Windows but still failing on Linux due to unrelated errors. I will add UTs and run sanity tests locally and then we can merge this.

Signed-off-by: Jiaping Zeng <[email protected]>

jiapingzeng · 2025-10-21T23:59:50Z

Added UTs for combineChunks. Not possible to create UT for prepareRequest as chunk collection happens async.

However, ran the below sanity tests:

Verified that sample workflow from here still works: [FEATURE] Agent Execute Stream #4212
Tested chunked request:

(
    echo -n '{"parameters":'
    sleep 0.1
    echo -n '{"question":'
    sleep 0.1
    echo -n '"What is'
    sleep 0.1
    echo -n ' the status'
    sleep 0.1
    echo -n ' of my'
    sleep 0.1
    echo -n ' cluster?"}}'
) > "$FIFO" &

curl -N -X POST "${OPENSEARCH_URL}/_plugins/_ml/agents/${AGENT_ID}/_execute/stream" \
  -H 'Content-Type: application/json' \
  -H 'Transfer-Encoding: chunked' \
  --data-binary @"$FIFO"

mingshl · 2025-10-22T00:52:12Z

plugin/src/main/java/org/opensearch/ml/rest/RestMLExecuteStreamAction.java

+            return BytesReference.fromByteBuffer(ByteBuffer.wrap(buffer.toByteArray()));
+        } catch (IOException e) {
+            log.error("Failed to combine chunks", e);
+            throw new OpenSearchStatusException("Failed to combine request chunks", RestStatus.INTERNAL_SERVER_ERROR, e);


We spoke about not using this OpenSearchStatusException as it will throw 500 errors. Have you tested on your end?

We can define HTTP error code in OpenSearchStatusException.
I see here we define it as 500 error. @mingshl Do you have any concern? Should we define as some other error code ?

I assume this is Server-side I/O failure while handling a request. If that's the case, then 500 should be the right status code.

I think 500 makes sense here as the chunks are failing to be combined which is I/O failure.

However when testing with invalid JSON once all the chunks are combined this 400 doesn't seem to be getting through. Will enhance error handling on this part instead.

return Mono.error(new OpenSearchStatusException("Failed to parse request", RestStatus.BAD_REQUEST, e));

jiapingzeng · 2025-10-22T19:55:53Z

Error handling sample requests:

curl -v -X POST "${OPENSEARCH_URL}/_plugins/_ml/agents/${AGENT_ID}/_execute/stream" \
  -H 'Content-Type: application/json' \
  -d '{"parameters":{"question":"test"'

data: {"error": "Failed to parse request: Unexpected end-of-input: expected close marker for Object (start marker at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 15])
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 33]"}

curl -v -X POST "${OPENSEARCH_URL}/_plugins/_ml/agents/${AGENT_ID}/_execute/stream" \
  -H 'Content-Type: application/json' \
  -d '{"parameters":::"question":"test"}}'

data: {"error": "Failed to parse request: Unexpected character (':' (code 58)): expected a valid value (JSON String, Number, Array, Object or token 'null', 'true' or 'false')
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 15]"}

FIFO=$(mktemp -u)
mkfifo "$FIFO"

(
    echo -n '{"parameters":'
    sleep 0.1
    echo -n '{"question":'
    sleep 0.1
    echo -n '"test"'
    # Missing closing braces
) > "$FIFO" &

curl -v -X POST "${OPENSEARCH_URL}/_plugins/_ml/agents/${AGENT_ID}/_execute/stream" \
  -H 'Content-Type: application/json' \
  -H 'Transfer-Encoding: chunked' \
  --data-binary @"$FIFO"

data: {"error": "Failed to parse request: Unexpected end-of-input: expected close marker for Object (start marker at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 15])
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 33]"}

Signed-off-by: Jiaping Zeng <[email protected]>

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 14, 2025 19:01 — with GitHub Actions Failure

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 14, 2025 19:01 — with GitHub Actions Error

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 14, 2025 19:01 — with GitHub Actions Failure

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 14, 2025 19:01 — with GitHub Actions Error

jiapingzeng marked this pull request as ready for review October 15, 2025 03:26

jiapingzeng requested review from HenryL27, Zhangxunmt, austintlee, b4sjoo, dhrubo-os, jngz-es, mingshl, model-collapse, pyek-bot, rbhavna, sam-herman, xinyual, ylwu-amzn and zane-neo as code owners October 15, 2025 03:26

ylwu-amzn force-pushed the execute-stream-chunk branch from 84ea119 to 8292d74 Compare October 15, 2025 17:40

ylwu-amzn temporarily deployed to ml-commons-cicd-env-require-approval October 15, 2025 17:42 — with GitHub Actions Inactive

ylwu-amzn had a problem deploying to ml-commons-cicd-env-require-approval October 15, 2025 17:42 — with GitHub Actions Error

ylwu-amzn had a problem deploying to ml-commons-cicd-env-require-approval October 15, 2025 17:42 — with GitHub Actions Failure

ylwu-amzn requested a deployment to ml-commons-cicd-env-require-approval October 15, 2025 20:31 — with GitHub Actions Waiting

combine json chunks

76d0400

Signed-off-by: Jiaping Zeng <[email protected]>

jiapingzeng force-pushed the execute-stream-chunk branch from 0a7734d to 76d0400 Compare October 21, 2025 23:07

jiapingzeng temporarily deployed to ml-commons-cicd-env-require-approval October 21, 2025 23:10 — with GitHub Actions Inactive

jiapingzeng dismissed stale reviews from ylwu-amzn and mingshl via 2a767b6 October 21, 2025 23:51

add unit tests

8f6c270

Signed-off-by: Jiaping Zeng <[email protected]>

jiapingzeng force-pushed the execute-stream-chunk branch from 2a767b6 to 8f6c270 Compare October 21, 2025 23:52

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 21, 2025 23:54 — with GitHub Actions Error

jiapingzeng temporarily deployed to ml-commons-cicd-env-require-approval October 21, 2025 23:54 — with GitHub Actions Inactive

jiapingzeng had a problem deploying to ml-commons-cicd-env-require-approval October 21, 2025 23:54 — with GitHub Actions Failure

jiapingzeng temporarily deployed to ml-commons-cicd-env-require-approval October 21, 2025 23:54 — with GitHub Actions Inactive

nathaliellenaa approved these changes Oct 22, 2025

View reviewed changes

mingshl reviewed Oct 22, 2025

View reviewed changes

jiapingzeng temporarily deployed to ml-commons-cicd-env-require-approval October 22, 2025 17:31 — with GitHub Actions Inactive

jiapingzeng requested a deployment to ml-commons-cicd-env-require-approval October 22, 2025 18:42 — with GitHub Actions Waiting

enhance input error handling

2bc0ae1

Signed-off-by: Jiaping Zeng <[email protected]>

jiapingzeng force-pushed the execute-stream-chunk branch from 291315f to 2bc0ae1 Compare October 22, 2025 20:20

jiapingzeng requested a deployment to ml-commons-cicd-env-require-approval October 22, 2025 20:23 — with GitHub Actions Waiting

peterzhuamazon mentioned this pull request Oct 23, 2025

[RELEASE] Release version 3.3.2 opensearch-project/opensearch-build#5799

Open

65 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

combine json chunks from requests #4317

combine json chunks from requests #4317

jiapingzeng commented Oct 14, 2025 •

edited

Loading

Uh oh!

nathaliellenaa commented Oct 14, 2025

Uh oh!

jiapingzeng commented Oct 14, 2025

Uh oh!

jiapingzeng commented Oct 15, 2025

Uh oh!

codecov bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

jiapingzeng commented Oct 21, 2025

Uh oh!

jiapingzeng commented Oct 21, 2025

Uh oh!

mingshl Oct 22, 2025

Uh oh!

ylwu-amzn Oct 22, 2025

Uh oh!

dhrubo-os Oct 22, 2025

Uh oh!

jiapingzeng Oct 22, 2025

Uh oh!

jiapingzeng commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

combine json chunks from requests #4317

Are you sure you want to change the base?

combine json chunks from requests #4317

Conversation

jiapingzeng commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

nathaliellenaa commented Oct 14, 2025

Uh oh!

jiapingzeng commented Oct 14, 2025

Uh oh!

jiapingzeng commented Oct 15, 2025

Uh oh!

codecov bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jiapingzeng commented Oct 21, 2025

Uh oh!

jiapingzeng commented Oct 21, 2025

Uh oh!

mingshl Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

ylwu-amzn Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

dhrubo-os Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

jiapingzeng Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

jiapingzeng commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jiapingzeng commented Oct 14, 2025 •

edited

Loading

codecov bot commented Oct 15, 2025 •

edited

Loading